-
Notifications
You must be signed in to change notification settings - Fork 5k
[Event Hub] PartitionResolver Jenkin3 optimizations #50068
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Thank you for your contribution @danielmarbach! We will review the pull request and get back to you soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR optimizes the Jenkin3 lookup hash computation in the partition key resolver by rewriting the 12-byte mix loop and streamlining tail handling.
- Replaces slice/indexed access with a single ref byte pointer and Unsafe.ReadUnaligned calls
- Streamlines tail processing using pointer arithmetic without bounds checks
- Eliminates intermediate ReadOnlySpan casts and allocations
sdk/eventhub/Azure.Messaging.EventHubs/src/Core/PartitionResolver.cs
Outdated
Show resolved
Hide resolved
d0873e0
to
6b81a2d
Compare
Thanks, @danielmarbach! Impressive work, as usual. Not gonna lie, though, the unaligned reads do scare me a bit. Don't suppose that you were able to do any validation passes on any Apple silicon or ARM platforms? I confirmed our macOS test legs in the pipeline are Intel-based, so I'm going to set up an ARM test for Monday if you didn't happen to already do one. |
@jsquire I ran all the existing tests on my M3 macbook pro if that counts |
It does indeed! If everything is passing on Apple silicon, I don't anticipate we'll run into an issues with ARM64 in Monday's pass. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like we've got test passes from runs on x64, ARM64, and Apple Silicon. Thanks, @danielmarbach!
Do you mind just throwing a change log entry in before we merge?
@jsquire done |
As discussed with @jsquire this optimizes the Jenkin3 lookup hash computation on the partition key resolver. I kept the hashing part as good as possible as is.
Unsafe.ReadUnaligned<uint>
instead ofSpan<byte>.Slice
or indexed accessReadOnlySpan<uint>
casts and allocations in the loop, preserving exact bit-for-bit Jenkins lookup3 outputResults
Benchmark